Refresh documentation navigation#2711
Conversation
|
Overall readability score: 53.93 (🔴 -0.06)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 high (1 false positive) |
🟢 Metrics 21 complexity · 0 duplication
Metric Results Complexity 21 Duplication 0
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request reorganizes the documentation structure and introduces a new tabbed navigation layout with updated responsive styling. The feedback focuses on optimizing a resize event listener in JavaScript to prevent layout thrashing, and simplifying redundant direction-specific CSS rules across several stylesheets by using modern logical properties like margin-inline and padding-inline-end.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function closeDrawerForPersistentLayout() { | ||
| if (!window.matchMedia("(min-width: 60em)").matches) { | ||
| return; | ||
| } | ||
|
|
||
| var drawerToggle = document.querySelector("#__drawer"); | ||
| var drawerTrigger = document.querySelector("[data-drawer-trigger]"); | ||
| if (drawerToggle && drawerToggle.checked) { | ||
| drawerToggle.checked = false; | ||
| drawerToggle.dispatchEvent(new Event("change")); | ||
| } | ||
|
|
||
| syncBackgroundInertState(); | ||
| if (drawerTrigger) { | ||
| drawerTrigger.setAttribute("aria-expanded", "false"); | ||
| drawerTrigger.setAttribute("aria-label", "Open navigation"); | ||
| } | ||
| } |
There was a problem hiding this comment.
The closeDrawerForPersistentLayout function is registered as a resize event listener. Since resize events fire rapidly during window resizing, executing DOM queries, querySelectorAll, and attribute modifications on every tick can cause significant layout thrashing and performance degradation (jank). Since we only need to reset the layout when the drawer is actually open, we can return early if the drawer is not checked.
function closeDrawerForPersistentLayout() {
if (!window.matchMedia("(min-width: 60em)").matches) {
return;
}
var drawerToggle = document.querySelector("#__drawer");
if (!drawerToggle || !drawerToggle.checked) {
return;
}
var drawerTrigger = document.querySelector("[data-drawer-trigger]");
drawerToggle.checked = false;
drawerToggle.dispatchEvent(new Event("change"));
syncBackgroundInertState();
if (drawerTrigger) {
drawerTrigger.setAttribute("aria-expanded", "false");
drawerTrigger.setAttribute("aria-label", "Open navigation");
}
}| [dir="ltr"] .md-content__inner { margin-left: var(--docs-content-inset); margin-right: var(--docs-content-inset); } | ||
| [dir="rtl"] .md-content__inner { margin-right: var(--docs-content-inset); margin-left: var(--docs-content-inset); } | ||
| [dir="ltr"] .md-path { margin-left: var(--docs-content-inset); margin-right: var(--docs-content-inset); } | ||
| [dir="rtl"] .md-path { margin-right: var(--docs-content-inset); margin-left: var(--docs-content-inset); } |
There was a problem hiding this comment.
Since both the LTR and RTL rules set both the left and right margins to the same value (var(--docs-content-inset)), we can simplify these rules into a single, direction-independent declaration using the CSS logical property margin-inline. This is cleaner, more modern, and consistent with the existing use of margin-inline and padding-inline in this file.
| [dir="ltr"] .md-content__inner { margin-left: var(--docs-content-inset); margin-right: var(--docs-content-inset); } | |
| [dir="rtl"] .md-content__inner { margin-right: var(--docs-content-inset); margin-left: var(--docs-content-inset); } | |
| [dir="ltr"] .md-path { margin-left: var(--docs-content-inset); margin-right: var(--docs-content-inset); } | |
| [dir="rtl"] .md-path { margin-right: var(--docs-content-inset); margin-left: var(--docs-content-inset); } | |
| .md-content__inner { margin-inline: var(--docs-content-inset); } | |
| .md-path { margin-inline: var(--docs-content-inset); } |
| [dir="ltr"] .md-sidebar--primary .md-nav__link, | ||
| [dir="rtl"] .md-sidebar--primary .md-nav__link { margin: 0; } |
There was a problem hiding this comment.
Since both LTR and RTL rules apply the exact same margin: 0 to .md-sidebar--primary .md-nav__link, the direction-specific selectors are redundant. We can simplify this to a single selector.
| [dir="ltr"] .md-sidebar--primary .md-nav__link, | |
| [dir="rtl"] .md-sidebar--primary .md-nav__link { margin: 0; } | |
| .md-sidebar--primary .md-nav__link { margin: 0; } |
| [dir="ltr"] .md-sidebar--primary .md-sidebar__inner { padding-right: 0; } | ||
| [dir="rtl"] .md-sidebar--primary .md-sidebar__inner { padding-left: 0; } | ||
|
|
||
| .md-sidebar--primary .md-sidebar__inner, | ||
| .md-sidebar--primary .md-nav--primary { width: 100%; max-width: none; box-sizing: border-box; } | ||
| .md-sidebar--primary .md-sidebar__inner { width: 100%; height: 100%; max-width: none; box-sizing: border-box; } | ||
| .md-sidebar--primary .md-nav--primary { width: 100%; height: auto; min-height: 100%; max-width: none; box-sizing: border-box; } | ||
| [dir="ltr"] .md-sidebar--secondary .md-sidebar__inner { padding-right: 2rem; } | ||
| [dir="rtl"] .md-sidebar--secondary .md-sidebar__inner { padding-left: 2rem; } |
There was a problem hiding this comment.
We can simplify these direction-specific padding rules by using the CSS logical property padding-inline-end. This eliminates the need for separate [dir="ltr"] and [dir="rtl"] selectors, making the stylesheet cleaner and more maintainable.
| [dir="ltr"] .md-sidebar--primary .md-sidebar__inner { padding-right: 0; } | |
| [dir="rtl"] .md-sidebar--primary .md-sidebar__inner { padding-left: 0; } | |
| .md-sidebar--primary .md-sidebar__inner, | |
| .md-sidebar--primary .md-nav--primary { width: 100%; max-width: none; box-sizing: border-box; } | |
| .md-sidebar--primary .md-sidebar__inner { width: 100%; height: 100%; max-width: none; box-sizing: border-box; } | |
| .md-sidebar--primary .md-nav--primary { width: 100%; height: auto; min-height: 100%; max-width: none; box-sizing: border-box; } | |
| [dir="ltr"] .md-sidebar--secondary .md-sidebar__inner { padding-right: 2rem; } | |
| [dir="rtl"] .md-sidebar--secondary .md-sidebar__inner { padding-left: 2rem; } | |
| .md-sidebar--primary .md-sidebar__inner { padding-inline-end: 0; } | |
| .md-sidebar--primary .md-sidebar__inner { width: 100%; height: 100%; max-width: none; box-sizing: border-box; } | |
| .md-sidebar--primary .md-nav--primary { width: 100%; height: auto; min-height: 100%; max-width: none; box-sizing: border-box; } | |
| .md-sidebar--secondary .md-sidebar__inner { padding-inline-end: 2rem; } |
| [dir="ltr"] .md-sidebar--primary .md-sidebar__inner { padding-right: .75rem; } | ||
| [dir="rtl"] .md-sidebar--primary .md-sidebar__inner { padding-left: .75rem; } |
There was a problem hiding this comment.
Similarly, we can simplify these direction-specific padding rules using padding-inline-end to avoid redundant LTR/RTL selectors.
| [dir="ltr"] .md-sidebar--primary .md-sidebar__inner { padding-right: .75rem; } | |
| [dir="rtl"] .md-sidebar--primary .md-sidebar__inner { padding-left: .75rem; } | |
| .md-sidebar--primary .md-sidebar__inner { padding-inline-end: .75rem; } |
Summary
Validation